-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(react-query): resolve hydration mismatch in SSR with prefetched queries #9572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a non-mutating server-view snapshot method to QueryObserver, switches useBaseQuery's useSyncExternalStore snapshot to that server view for SSR hydration, and adds extensive tests validating server vs. client snapshots across pending, fetching, error, and concurrency scenarios. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as React Component
participant H as useBaseQuery
participant O as QueryObserver
participant QC as QueryCache
C->>H: render()
H->>O: create/attach observer
H->>H: useSyncExternalStore(subscribe, getServerSnapshot)
H->>O: getServerResult()
O->>QC: read query state (dataUpdatedAt, fetchStatus)
alt dataUpdatedAt == 0 (hydrated)
Note right of O: build synthetic server view\n(status: pending / isPending: true)
O-->>H: serverResult (pending-like, data undefined or masked)
else
O-->>H: currentResult (unchanged)
end
H-->>C: snapshot for render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
View your CI Pipeline Execution ↗ for commit 3429d75
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/react-query/src/useBaseQuery.ts (1)
111-113
: Optionally harden against version skew between @tanstack/react-query and @tanstack/query-coreIf a consumer upgrades react-query without upgrading query-core,
observer.getServerResult
may not exist at runtime. A small runtime fallback guards against that without affecting current behavior.Apply this diff:
), () => observer.getCurrentResult(), - () => observer.getServerResult(), + () => + // Guard against version skew where older query-core doesn't have getServerResult + (observer as any).getServerResult + ? (observer as any).getServerResult() + : observer.getCurrentResult(), )packages/query-core/src/queryObserver.ts (2)
274-287
: Consider normalizing “pending” snapshot flags for full internal consistencyWhen returning a synthetic pending result,
isFetched
/isFetchedAfterMount
could still reflect “fetched” from the spread. While likely harmless for SSR, explicitly setting them tofalse
would avoid mixed semantics (“pending but fetched”).Apply this diff:
const pendingResult: QueryObserverResult<TData, TError> = { ...currentResult, status: 'pending', isPending: true, isSuccess: false, isError: false, isLoading: currentResult.fetchStatus === 'fetching', isInitialLoading: currentResult.fetchStatus === 'fetching', data: undefined, error: null, isLoadingError: false, isRefetchError: false, isPlaceholderData: false, + // Normalize fetched flags in the synthetic pending server snapshot + isFetched: false, + isFetchedAfterMount: false, } as QueryObserverResult<TData, TError>
267-269
: Add a short JSDoc explaining server snapshot semanticsA brief doc comment will clarify why
dataUpdatedAt === 0
is treated as hydrated and why we synthesize a pending result.Apply this diff:
- getServerResult(): QueryObserverResult<TData, TError> { + /** + * Returns a "server-view" snapshot for useSyncExternalStore during SSR. + * If the observer has hydrated client data (dataUpdatedAt === 0) but the server + * should render a pending UI, synthesize a pending result; otherwise, return the current result. + */ + getServerResult(): QueryObserverResult<TData, TError> {packages/query-core/src/__tests__/queryObserver.test.tsx (1)
1498-1523
: Deduplicate “mark as hydrated” setup with a small helperMultiple tests inline the same mutation of
dataUpdatedAt
andfetchStatus
. A helper will reduce repetition and make intent clearer.An example helper to insert right after the “SSR Hydration” describe:
describe('SSR Hydration', () => { + const markHydrated = ( + qc: QueryClient, + key: ReturnType<typeof queryKey>, + data: unknown, + fetchStatus: 'idle' | 'fetching' = 'idle', + ) => { + qc.setQueryData(key, data) + const cache = qc.getQueryCache().find({ queryKey: key }) + if (cache) { + cache.state.dataUpdatedAt = 0 + cache.state.fetchStatus = fetchStatus + } + }Then, for example:
- queryClient.setQueryData(key, { amount: 10 }) - const cache = queryClient.getQueryCache().find({ queryKey: key }) - if (cache) { - cache.state.dataUpdatedAt = 0 - cache.state.fetchStatus = 'idle' - } + markHydrated(queryClient, key, { amount: 10 }, 'idle')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/query-core/src/__tests__/queryObserver.test.tsx
(1 hunks)packages/query-core/src/queryObserver.ts
(1 hunks)packages/react-query/src/useBaseQuery.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/types.ts (1)
QueryObserverResult
(899-904)
packages/react-query/src/useBaseQuery.ts (1)
packages/query-core/src/queriesObserver.ts (1)
observer
(254-260)
packages/query-core/src/__tests__/queryObserver.test.tsx (2)
packages/query-core/src/queriesObserver.ts (1)
observer
(254-260)packages/query-core/src/queryObserver.ts (1)
QueryObserver
(39-765)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (3)
packages/react-query/src/useBaseQuery.ts (1)
112-113
: Switching server snapshot to getServerResult fixes hydration mismatch — LGTMThis is the right hook for SSR to align the server snapshot with server-rendered HTML under hydration. No behavior changes for client snapshots.
packages/query-core/src/queryObserver.ts (1)
267-294
: Well-scoped server snapshot that models hydrated data as pending — LGTMNon-mutating, minimal, and conditionally applied only under the hydrated-success state (
dataUpdatedAt === 0
). Keepspromise
intact and mirrors fetchStatus for loading flags. This aligns withuseSyncExternalStore
’s SSR needs.packages/query-core/src/__tests__/queryObserver.test.tsx (1)
1498-1671
: Solid coverage of hydration cases and server/client snapshot divergence — LGTMThe tests clearly demonstrate the mismatch and validate the pending server snapshot across idle/fetching/error paths, plus divergence from client snapshots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this 🙌
status: 'pending', | ||
isPending: true, | ||
isSuccess: false, | ||
isError: false, | ||
isLoading: currentResult.fetchStatus === 'fetching', | ||
isInitialLoading: currentResult.fetchStatus === 'fetching', | ||
data: undefined, | ||
error: null, | ||
isLoadingError: false, | ||
isRefetchError: false, | ||
isPlaceholderData: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how did you come up with this list of fields to change? For example, why aren’t fields like isFetched
or isStale
being re-set?
I’m wondering how we can make sure to keep this list in-sync if we add new properties to QueryObserverResult
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback!
In conclusion, the criteria for selecting the specified fields was “only change fields that directly affect SSR hydration inconsistencies.”
I believe that intentionally maintaining the currentResult value for the remaining fields will be beneficial during subsequent maintenance processes.
However, based on coderabbitai's feedback, I realized that I had overlooked isRefetching
, so I added it.
Since the status is fixed as pending
, I added it to explicitly state that refetching is not possible.
currentResult.status === 'success' && | ||
this.#currentQuery.state.dataUpdatedAt === 0 | ||
) { | ||
const pendingResult: QueryObserverResult<TData, TError> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be of type QueryObserverPendingResult
to make sure we’re at least always in-sync with what this type guarantees ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing out the type > QueryObserverPendingResult
, which I had overlooked.
I agree that applying this will improve type stability.
However, since currentResult
must be of type QueryObserverResult
, I handled it as a union.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/query-core/src/queryObserver.ts (1)
277-288
: Avoid brittle “whitelist” of overridden fields; centralize pending server result construction.Manually overriding a subset of properties is easy to get out of sync with
QueryObserverResult
as new fields are added (e.g.,isFetched
,isRefetching
was already missed). Consider extracting a small helper to construct a coherent pending server snapshot fromcurrentResult
andthis.#currentQuery.state.fetchStatus
, which sets all invariants in one place and is typed asQueryObserverPendingResult
.I can draft a
private toServerPendingResult(...)
helper to keep this maintainable if helpful.
🧹 Nitpick comments (1)
packages/query-core/src/queryObserver.ts (1)
268-271
: Simplify return type: QueryObserverPendingResult is already part of QueryObserverResult.
QueryObserverPendingResult
is a constituent ofQueryObserverResult
, so the explicit union is redundant. Keeping the return type as justQueryObserverResult<TData, TError>
reduces noise.- getServerResult(): - | QueryObserverPendingResult<TData, TError> - | QueryObserverResult<TData, TError> { + getServerResult(): QueryObserverResult<TData, TError> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/query-core/src/queryObserver.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/types.ts (2)
QueryObserverPendingResult
(797-810)QueryObserverResult
(899-904)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
getServerResult(): | ||
| QueryObserverPendingResult<TData, TError> | ||
| QueryObserverResult<TData, TError> { | ||
const currentResult = this.#currentResult | ||
|
||
const isHydratedData = | ||
currentResult.status === 'success' && | ||
this.#currentQuery.state.dataUpdatedAt === 0 | ||
|
||
if (isHydratedData) { | ||
return { | ||
...currentResult, | ||
status: 'pending' as const, | ||
isPending: true, | ||
isSuccess: false, | ||
data: undefined, | ||
isLoading: currentResult.fetchStatus === 'fetching', | ||
isInitialLoading: currentResult.fetchStatus === 'fetching', | ||
isPlaceholderData: false, | ||
} | ||
} | ||
|
||
return currentResult | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix inconsistent flags for pending server snapshot (isRefetching, error flags, fetched flags).
When transforming a hydrated success result into a pending server snapshot, we spread currentResult
and override a few fields. This leaves inconsistencies:
- If
fetchStatus === 'fetching'
,currentResult.isRefetching
wastrue
(because success + fetching), but a pending result must haveisRefetching: false
(as computed in createResult:isRefetching = isFetching && !isPending
). - Pending semantics also guarantee
isError: false
,error: null
,isLoadingError: false
,isRefetchError: false
, and it’s safer to markisFetched
/isFetchedAfterMount
asfalse
to avoid mixed semantics leaking from the success snapshot.
Recommend explicitly overriding these to match QueryObserverPendingResult
invariants and make the branch type-safe.
Apply this diff inside getServerResult
:
if (isHydratedData) {
- return {
- ...currentResult,
- status: 'pending' as const,
- isPending: true,
- isSuccess: false,
- data: undefined,
- isLoading: currentResult.fetchStatus === 'fetching',
- isInitialLoading: currentResult.fetchStatus === 'fetching',
- isPlaceholderData: false,
- }
+ return {
+ ...currentResult,
+ status: 'pending' as const,
+ isPending: true,
+ isSuccess: false,
+ // pending invariants
+ isError: false,
+ error: null,
+ isLoadingError: false,
+ isRefetchError: false,
+ // data is not available for pending
+ data: undefined,
+ // compute loading flags from fetchStatus
+ isLoading: currentResult.fetchStatus === 'fetching',
+ isInitialLoading: currentResult.fetchStatus === 'fetching',
+ // pending cannot be "refetching"
+ isRefetching: false,
+ // do not report placeholder data
+ isPlaceholderData: false,
+ // avoid leaking fetched flags from the success snapshot
+ isFetched: false,
+ isFetchedAfterMount: false,
+ } as QueryObserverPendingResult<TData, TError>
}
🤖 Prompt for AI Agents
In packages/query-core/src/queryObserver.ts around lines 268 to 291, when
converting a hydrated success result into a pending server snapshot we currently
spread currentResult but leave several flags inconsistent; explicitly override
the following fields to match QueryObserverPendingResult invariants: set
isRefetching to false, isError to false, error to null, isLoadingError to false,
isRefetchError to false, isFetched to false, and isFetchedAfterMount to false
(retain the existing overrides for
status/isPending/isSuccess/data/isLoading/isInitialLoading/isPlaceholderData).
Ensure these fields are explicitly assigned in the returned object so the
pending snapshot has consistent semantics and is type-safe.
💡 Verification agent
🧩 Analysis chain
Add test coverage for isRefetching and fetched flags in server snapshot.
To prevent regressions, assert in tests that:
getServerResult()
for hydrated data setsisRefetching === false
even iffetchStatus === 'fetching'
.isFetched === false
andisFetchedAfterMount === false
.
[offer_assistance]
If missing, I can provide a focused test case to assert these invariants.
🏁 Script executed:
#!/bin/bash
# Locate existing tests around getServerResult and hydration
rg -n -C3 "getServerResult|server snapshot|hydration" --type=ts --type=tsx
# Check if isRefetching/isFetched assertions exist
rg -n -C2 "isRefetching|isFetchedAfterMount|isFetched" packages/query-core/src/__tests__/queryObserver.test.tsx
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Retry pattern search without relying on --type (tsx not recognized)
rg -n -C3 "getServerResult|server snapshot|hydration" -g '*.ts' -g '*.tsx'
rg -n -C2 "isRefetching|isFetchedAfterMount|isFetched" -g '*.ts' -g '*.tsx'
Length of output: 120039
Add missing assertions for isRefetching
, isFetched
, and isFetchedAfterMount
in server snapshot tests
The existing hydration tests in packages/query-core/src/__tests__/queryObserver.test.tsx
verify status
, data
, and fetchStatus
, but don’t cover these derived flags. To prevent regressions, extend the tests under the Solution with getServerResult
block:
• After the “should return pending state for hydrated data” test (around line 1530), add:
expect(serverResult.isFetched).toBe(false)
expect(serverResult.isFetchedAfterMount).toBe(false)
expect(serverResult.isRefetching).toBe(false)
• After the “should handle fetching state during hydration” test (around line 1575), also assert:
expect(serverResult.isFetched).toBe(false)
expect(serverResult.isFetchedAfterMount).toBe(false)
expect(serverResult.isRefetching).toBe(false)
This will ensure that hydrated snapshots never report fetched or refetching states on the server.
🤖 Prompt for AI Agents
In packages/query-core/src/queryObserver.ts around lines 268 to 291: the server
result produced for hydrated data is missing corresponding unit-test assertions;
update the tests in packages/query-core/src/__tests__/queryObserver.test.tsx
(after the "should return pending state for hydrated data" test near line ~1530
and after the "should handle fetching state during hydration" test near line
~1575) to add assertions that serverResult.isFetched is false,
serverResult.isFetchedAfterMount is false, and serverResult.isRefetching is
false so hydrated server snapshots never report fetched or refetching states.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/query-core/src/queryObserver.ts (2)
277-293
: Pending server snapshot leaks fetched flags; align with pending invariants.When masking a hydrated success as pending,
isFetched
/isFetchedAfterMount
may still reflect the success snapshot due to the spread. Explicitly reset them tofalse
to avoid mixed semantics. Optionally, cast the object toQueryObserverPendingResult
for stronger type guarantees.Apply this diff within the pending branch:
if (isHydratedData) { return { ...currentResult, status: 'pending' as const, isPending: true, isSuccess: false, isError: false, error: null, isLoadingError: false, isRefetchError: false, data: undefined, isRefetching: false, isLoading: currentResult.fetchStatus === 'fetching', isInitialLoading: currentResult.fetchStatus === 'fetching', isPlaceholderData: false, + // avoid leaking "fetched" semantics from the success snapshot + isFetched: false, + isFetchedAfterMount: false, - } + } as QueryObserverPendingResult<TData, TError> }
277-293
: Add test assertions for fetched/refetch flags in server snapshot.To prevent regressions, extend the SSR/hydration tests to assert:
serverResult.isFetched === false
serverResult.isFetchedAfterMount === false
serverResult.isRefetching === false
This ensures the pending server snapshot never exposes “fetched” or “refetching” semantics.
🧹 Nitpick comments (1)
packages/query-core/src/queryObserver.ts (1)
273-276
: Sentinel usage confirmed, no unintended zero-timestamps detected
- dataUpdatedAt === 0 only appears as the default initial value in tests and is never reset to 0 elsewhere in your runtime code.
- The
isHydratedData
check inpackages/query-core/src/queryObserver.ts:273–276
therefore correctly identifies only “fresh” hydration cases.- No other code paths or hydration routines assign
dataUpdatedAt = 0
post-dehydration.Nitpick (optional): if you prefer to avoid sprinkling this magic number, you could extract a small helper (e.g.
isHydrated(result)
) to centralize the sentinel logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/query-core/src/queryObserver.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/types.ts (2)
QueryObserverPendingResult
(797-810)QueryObserverResult
(899-904)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/query-core/src/queryObserver.ts (2)
26-26
: LGTM: type-only import forQueryObserverPendingResult
.Using
import type
keeps this out of runtime bundles and is consistent with surrounding imports.
268-271
: Great addition: dedicated server snapshot API for useSyncExternalStore.Providing a server-specific snapshot via
getServerResult
cleanly separates SSR vs. client snapshots and addresses the hydration mismatch root cause without mutating observer state.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9572 +/- ##
===========================================
+ Coverage 45.15% 59.29% +14.14%
===========================================
Files 208 137 -71
Lines 8323 5570 -2753
Branches 1886 1502 -384
===========================================
- Hits 3758 3303 -455
+ Misses 4118 1963 -2155
+ Partials 447 304 -143 🚀 New features to boost your workflow:
|
I’m not an expert here so I’ll wait for @Ephem to jump in but the way I see it, the |
Hmm, I understand what you're saying. My PR currently checks the status of currentResult at runtime and changes it dynamically, so there is a possibility of inconsistency. The Context approach you suggested preserves the value rather than transforming it, which aligns more closely with React's philosophy. I'm also curious about @Ephem final opinion. For now, I see two paths forward
For now, I will wait for @Ephem response and refactor this PR using the Context method. +++
In other words, the “actual pending state created by the server” that we want to put into Context has never actually existed, has it? The server only knows the “success” state from start to finish, and the “pending” state we desire is merely our wish for how it should appear. Therefore, the Context approach ultimately // Not “preservation” but still “creation/transformation”
const serverSnapshot = {
status: ‘pending’, // artificially created
data: undefined, // hiding the actual data
} This is an artificially created state being placed into the Context, not preserving the actual server state. |
I added comprehensive test cases to verify the issues you raised and obtained the following results. Test 1 Test 2 Test 3 // Before
isLoading: currentResult.fetchStatus === ‘fetching’,
// After
isLoading: queryState.fetchStatus === ‘fetching’, This change ensures that the real-time fetch status is reflected even during concurrent operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/query-core/src/__tests__/queryObserver.test.tsx (4)
1503-1508
: Avoid mutatingquery.state
directly; usequery.setState
to keep invariants and notifications intactThese tests directly mutate
cache.state.{dataUpdatedAt,fetchStatus}
. This bypassesQuery
’s state machinery and can make tests brittle against internal changes (e.g., observers not being notified, internal derived fields not recomputed). Prefercache.setState(...)
.Example fix for Lines 1503–1508:
- if (cache) { - cache.state.dataUpdatedAt = 0 - cache.state.fetchStatus = 'idle' - } + if (cache) { + cache.setState({ dataUpdatedAt: 0, fetchStatus: 'idle' }) + }Similarly adjust Lines 1562–1567:
- if (cache) { - cache.state.dataUpdatedAt = 0 - cache.state.fetchStatus = 'fetching' - } + if (cache) { + cache.setState({ dataUpdatedAt: 0, fetchStatus: 'fetching' }) + }Lines 1586–1590:
- if (cache) { - cache.state.dataUpdatedAt = Date.now() - } + if (cache) { + cache.setState({ dataUpdatedAt: Date.now() }) + }Lines 1712–1717:
- if (cache) { - cache.state.dataUpdatedAt = 0 - cache.state.fetchStatus = 'fetching' - } + if (cache) { + cache.setState({ dataUpdatedAt: 0, fetchStatus: 'fetching' }) + }Lines 1740–1745:
- if (cache) { - cache.state.dataUpdatedAt = 0 - cache.state.fetchStatus = 'idle' - } + if (cache) { + cache.setState({ dataUpdatedAt: 0, fetchStatus: 'idle' }) + }Also applies to: 1562-1567, 1586-1590, 1712-1717, 1740-1745
1501-1501
: Clarify test title to reflect what is assertedThis test doesn’t actually demonstrate a hydration “mismatch”; it asserts the client result is “success”. Consider renaming to make intent explicit:
- test('should demonstrate hydration mismatch issue (before fix)', () => { + test('client snapshot with hydrated data is success (illustrates mismatch risk pre-fix)', () => {
1680-1685
: Reconsider the “future-proof status” test usingstatus: 'idle' as any
Injecting an out-of-contract status via
as any
couples the test to speculation about future versions and may confuse readers because it asserts that data can exist with an "idle" status. If the goal is to document thatgetServerResult
only masks whenstatus === 'success'
, make that explicit instead, or mark this as a documentation-style test with a clear comment.
- Option A: Drop the test to avoid validating undefined behavior.
- Option B: Keep it, but add a prominent comment explaining it asserts current, intentional masking criteria.
1656-1669
: Optional: Add an end-to-end React test inpackages/react-query
to exerciseuseSyncExternalStore
snapshot parityThis unit test checks the two snapshots via
getCurrentResult
andgetServerResult
, which is good. For higher confidence, consider adding an integration test inpackages/react-query
that mounts a tiny component usinguseBaseQuery
and verifies that the server snapshot (from uSESgetServerSnapshot
) matches the HTML and that hydration does not mismatch when client data resolves.Happy to help draft that test if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/query-core/src/__tests__/queryObserver.test.tsx
(1 hunks)packages/query-core/src/queryObserver.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/query-core/src/queryObserver.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/query-core/src/__tests__/queryObserver.test.tsx (3)
packages/query-core/src/queriesObserver.ts (1)
observer
(254-260)packages/query-core/src/queryObserver.ts (1)
QueryObserver
(40-770)packages/query-core/src/utils.ts (1)
sleep
(355-359)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
cache.state.dataUpdatedAt = Date.now() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Use a non-zero constant (or set system time) to avoid flakiness with fake timers
Under fake timers, Date.now() may be 0 unless you set the system time. That would unintentionally mark this as “hydrated” (dataUpdatedAt === 0), causing getServerResult
to mask the result and fail the “non-hydrated” expectations.
Minimal, deterministic fix:
- cache.state.dataUpdatedAt = Date.now()
+ cache.setState({ dataUpdatedAt: 1 })
Alternatively, set the mocked clock first:
vi.setSystemTime(1)
cache.setState({ dataUpdatedAt: Date.now() })
🏁 Script executed:
#!/bin/bash
# Locate usages of dataUpdatedAt to see surrounding test logic
rg -n "dataUpdatedAt" -C3 packages/query-core/src/__tests__/queryObserver.test.tsx
# Find direct Date.now() calls in the test file
rg -n "Date.now" -C3 packages/query-core/src/__tests__/queryObserver.test.tsx
# Check for any mocked clock setup or system time manipulation
rg -n "setSystemTime" -C3 packages/query-core/src/__tests__/queryObserver.test.tsx
# Search for fake-timer setup (vi.useFakeTimers or jest.useFakeTimers)
rg -nP "useFakeTimers|fakeTimers" -C3 packages/query-core/src/__tests__/queryObserver.test.tsx
Length of output: 3741
Use a non-zero constant for dataUpdatedAt
to avoid flakiness under fake timers
Under vi.useFakeTimers()
, Date.now()
returns 0
unless you explicitly advance the clock—making your “fresh” timestamp indistinguishable from the unhydrated state. Update the assignment in packages/query-core/src/__tests__/queryObserver.test.tsx
to use a non-zero literal (or advance the fake clock first):
• Minimal, consistent with other direct state mutations:
- cache.state.dataUpdatedAt = Date.now()
+ cache.state.dataUpdatedAt = 1
• Alternative (advance the fake clock, then read):
vi.setSystemTime(1)
cache.state.dataUpdatedAt = Date.now()
🤖 Prompt for AI Agents
In packages/query-core/src/__tests__/queryObserver.test.tsx around lines
1589-1590, the test sets cache.state.dataUpdatedAt = Date.now() while using
vi.useFakeTimers(), which makes Date.now() return 0 and causes flakiness; change
the assignment to use a non-zero literal (e.g. set dataUpdatedAt to 1) or
advance the fake timers first by calling vi.setSystemTime(1) and then assign
Date.now() so the "fresh" timestamp is non-zero and consistent under fake
timers.
fixes #9399
fixes #4690
Comment: #9399 (comment)
Description
This PR fixes a hydration mismatch issue that occurs when using
prefetchQuery
in Next.js App Router with React Query v5.The Problem
I briefly looked into the issue.
It's really interesting…
I added a few debugging logs to the reporter's code and checked the results.
I naturally expected that since the server prefetches and hydrates before sending to the client, if the server's
data
isundefined
, the client would also haveundefined
, and if the server has data, the client would have it as well.But the results were as follows
Sometimes it was
undefined
on the server, yet the client had data.I thought about why this might be possible.
Possible hypothesis =>
// 1. RSC (React Server Component) stage
Layout
page.tsx
(server component)→
getServerQueryClient()
creates QueryClient A→
prefetchQuery()
runs (queryFn execution "1")→
dehydrate(QueryClient A)
= serialize data// 2. SSR (Server Side Rendering) stage
ClientProvider
runs→
createQueryClient()
creates QueryClient B (new, empty cache!)PageClient
(uses QueryClient B)→ Cache is empty →
useQuery
starts fetch (queryFn execution "2")→
status: 'pending'
,isLoading: true
→ HTML:
<em>Loading...</em>
// 3. RSC payload generation
→ e.g.,
{"amount":14}
This is what I actually observed (you can find this injection in the Network tab > search for
amount
underlocalhost
)Then the client state becomes
// 1. Initial state
ClientProvider
: creates QueryClient C (new)HydrationBoundary
: receives the dehydrated state from QueryClient A// 2. During hydration
HydrationBoundary
'suseMemo
→ Finds new queries → hydrates immediately
→ Creates a Promise-like object
→ Fills it with data from the RSC payload
// 3. React Query v5
tryResolveSync
status: 'success'
,data: { amount: 14 }
// 4. First render
isLoading: false
"14"
<em>Loading...</em>
vs"14"
)In summary, the core points seem to be
QueryClient
instances are created independently, fragmenting state across them.tryResolveSync
behavior.The Solution
To resolve this hydration mismatch, I implemented
getServerSnapshot
support inuseSyncExternalStore
1. Added
getServerResult()
method to QueryObserverThis method detects hydrated data (where
dataUpdatedAt === 0
) and returns a pending state for server-side rendering2. Updated
useBaseQuery
to use proper server snapshotTesting
I've added comprehensive tests to verify the fix
All tests are passing
Build & Test Results
I've successfully run the following locally
pnpm test
- All tests passingpnpm build:all
- Build successfulpnpm test:types
- Type checks passingRelated Issues
getServerSnapshot
Breaking Changes
None. This is a backward-compatible fix that only affects SSR behavior.
Summary by CodeRabbit
Bug Fixes
Tests